bitkeeper revision 1.229 (3ec26728uXDBndxeN2-AxbIwi9afug)

dev.c, vif.h, skbuff.h, dom0_ops.c:
  Fixed locking in network transmit and receive. Fixed bug I added to DOM0_GETDOMAININFO in previous checkin :-)

diff --git a/xen/common/dom0_ops.c b/xen/common/dom0_ops.c
index 89872251f760b7219b7a4def10e2d01ecfc2048d..c62b68b919998407e49a4c8cee3fc5678ab1d04e 100644 (file)
--- a/xen/common/dom0_ops.c
+++ b/xen/common/dom0_ops.c
@@ -210,18 +210,20 @@ long do_dom0_op(dom0_op_t *u_dom0_op)
 
     case DOM0_GETDOMAININFO:
     { 
-        struct task_struct *p;
+        struct task_struct *p = &idle0_task;
         u_long flags;
 
-        p = idle0_task.next_task;
         read_lock_irqsave (&tasklist_lock, flags);
-        do {
-            if ((!is_idle_task (p)) && (p -> domain >= op.u.getdominfo.domain))
+
+        while ( (p = p->next_task) != &idle0_task )
+            if ( !is_idle_task(p) && (p->domain >= op.u.getdominfo.domain) )
                 break;
-        } while ((p = p -> next_task) != &idle0_task);
 
-        ret = -ESRCH;
-        if ( p != &idle0_task ) 
+        if ( p == &idle0_task )
+        {
+            ret = -ESRCH;
+        }
+        else
         {
             op.u.getdominfo.domain      = p->domain;
             strcpy (op.u.getdominfo.name, p->name);

diff --git a/xen/include/xeno/skbuff.h b/xen/include/xeno/skbuff.h
index 367a7c114e6837da18d88df2aeaa680ac7219edd..4e0afc018361ef77c67400d5e844992170089caf 100644 (file)
--- a/xen/include/xeno/skbuff.h
+++ b/xen/include/xeno/skbuff.h
@@ -156,10 +156,12 @@ struct sk_buff {
     unsigned char      *end;                   /* End pointer                                  */
 
     void               (*destructor)(struct sk_buff *);        /* Destruct function            */
-    struct pfn_info *pf;                    /* record of physical pf address for freeing    */
-    net_vif_t *src_vif;                     /* vif we came from                             */
-    net_vif_t *dst_vif;                     /* vif we are bound for                         */
-    struct skb_shared_info shinfo;          /* shared info is no longer shared in Xen.      */
+
+    unsigned short guest_id;  /* guest-OS's id for this packet (tx only!)   */
+    struct pfn_info *pf;      /* record of physical pf address for freeing  */
+    net_vif_t *src_vif;       /* vif we came from                           */
+    net_vif_t *dst_vif;       /* vif we are bound for                       */
+    struct skb_shared_info shinfo; /* shared info not shared in Xen.        */
 };
 
 extern void           __kfree_skb(struct sk_buff *skb);

diff --git a/xen/include/xeno/vif.h b/xen/include/xeno/vif.h
index 6cc339cbd9b8690e0c5ef4cb60648eb0ba3e5b18..8e4fcab444bbe4ff0cf1221798a48a66a3d1d331 100644 (file)
--- a/xen/include/xeno/vif.h
+++ b/xen/include/xeno/vif.h
@@ -58,8 +58,7 @@ typedef struct net_vif_st {
     unsigned int rx_cons;  /* Next buffer to fill is here. */
     tx_shadow_entry_t tx_shadow_ring[TX_RING_SIZE];
     unsigned int tx_prod;  /* More packets for sending go here. */
-    unsigned int tx_idx;   /* Next packet to send is here. */
-    unsigned int tx_cons;  /* Next packet to create response for is here. */
+    unsigned int tx_cons;  /* Next packet to send is here. */
 
     /* Private indexes into shared ring. */
     unsigned int rx_req_cons;

diff --git a/xen/net/dev.c b/xen/net/dev.c
index 0f36344e74979eb7959f36e50a1e693bf70276ee..ce2795f3a0986a8b7a0ee507aa5ebdf9eae44c0d 100644 (file)
--- a/xen/net/dev.c
+++ b/xen/net/dev.c
@@ -504,16 +504,24 @@ void deliver_packet(struct sk_buff *skb, net_vif_t *vif)
     if ( ntohs(skb->mac.ethernet->h_proto) == ETH_P_ARP )
         memcpy(skb->nh.raw + 18, vif->vmac, ETH_ALEN);
 
+    /*
+     * Slightly gross: we need the page_lock so that we can do PTE checking.
+     * However, we take it slightly early so that it can protect the update
+     * of rx_cons. This saves us from grabbing two locks.
+     */
+    spin_lock(&vif->domain->page_lock);
+
     if ( (i = vif->rx_cons) == vif->rx_prod )
+    {
+        spin_unlock(&vif->domain->page_lock);
         return;
-
+    }
     rx = vif->rx_shadow_ring + i;
+    vif->rx_cons = RX_RING_INC(i);
 
     size   = (unsigned short)skb->len;
     offset = (unsigned char)((unsigned long)skb->data & ~PAGE_MASK);
 
-    spin_lock(&vif->domain->page_lock);
-
     /* Release the page-table page. */
     pte_page = frame_table + (rx->pte_ptr >> PAGE_SHIFT);
     put_page_type(pte_page);
@@ -529,6 +537,8 @@ void deliver_packet(struct sk_buff *skb, net_vif_t *vif)
         /* Bail out if the PTE has been reused under our feet. */
         list_add(&old_page->list, &vif->domain->pg_head);
         old_page->flags = vif->domain->domain;
+        unmap_domain_mem(ptep);
+        spin_unlock(&vif->domain->page_lock);
         status = RING_STATUS_BAD_PAGE;
         goto out;
     }
@@ -558,13 +568,11 @@ void deliver_packet(struct sk_buff *skb, net_vif_t *vif)
      * NB. The remote flush here should be safe, as we hold no locks. The 
      * network driver that called us should also have no nasty locks.
      */
-    rx = vif->rx_shadow_ring + vif->rx_cons;
     if ( rx->flush_count == (unsigned short)
          atomic_read(&tlb_flush_count[vif->domain->processor]) )
         flush_tlb_cpu(vif->domain->processor);
 
  out:
-    vif->rx_cons = RX_RING_INC(vif->rx_cons);
     make_rx_response(vif, rx->id, size, status, offset);
 }
 
@@ -682,7 +690,6 @@ static void tx_skb_release(struct sk_buff *skb)
 {
     int i;
     net_vif_t *vif = skb->src_vif;
-    tx_shadow_entry_t *tx;
     unsigned long flags;
     
     spin_lock_irqsave(&vif->domain->page_lock, flags);
@@ -695,9 +702,7 @@ static void tx_skb_release(struct sk_buff *skb)
 
     skb_shinfo(skb)->nr_frags = 0; 
 
-    tx = vif->tx_shadow_ring + vif->tx_cons;
-    vif->tx_cons = TX_RING_INC(vif->tx_cons);
-    make_tx_response(vif, tx->id, RING_STATUS_OK);
+    make_tx_response(vif, skb->guest_id, RING_STATUS_OK);
 
     put_vif(vif);
 }
@@ -720,7 +725,7 @@ static void net_tx_action(unsigned long unused)
         vif = list_entry(ent, net_vif_t, list);
         get_vif(vif);
         remove_from_net_schedule_list(vif);
-        if ( vif->tx_idx == vif->tx_prod )
+        if ( vif->tx_cons == vif->tx_prod )
         {
             put_vif(vif);
             continue;
@@ -735,9 +740,9 @@ static void net_tx_action(unsigned long unused)
         }
         
         /* Pick an entry from the transmit queue. */
-        tx = &vif->tx_shadow_ring[vif->tx_idx];
-        vif->tx_idx = TX_RING_INC(vif->tx_idx);
-        if ( vif->tx_idx != vif->tx_prod )
+        tx = &vif->tx_shadow_ring[vif->tx_cons];
+        vif->tx_cons = TX_RING_INC(vif->tx_cons);
+        if ( vif->tx_cons != vif->tx_prod )
             add_to_net_schedule_list_tail(vif);
 
         skb->destructor = tx_skb_release;
@@ -749,6 +754,7 @@ static void net_tx_action(unsigned long unused)
         skb->src_vif  = vif;
         skb->dst_vif  = NULL;
         skb->mac.raw  = skb->data; 
+        skb->guest_id = tx->id;
         
         skb_shinfo(skb)->frags[0].page        = frame_table +
             (tx->payload >> PAGE_SHIFT);